Skip to content

fix(video): return pathlib.Path from Video.path() - #3197

Open
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/video-path-returns-path
Open

fix(video): return pathlib.Path from Video.path()#3197
Mohammed Alkindi (MohammedAlkindi) wants to merge 1 commit into
microsoft:mainfrom
MohammedAlkindi:fix/video-path-returns-path

Conversation

@MohammedAlkindi

Copy link
Copy Markdown

Video.path() is annotated -> pathlib.Path and the generated API documents Returns: pathlib.Path, but it returns Artifact.absolute_path, which is the raw initializer["absolutePath"] string.

The wrap was dropped in 0c70108 ("roll to ... aka video", Apr 2021) when Video moved onto the Artifact abstraction. Before that commit it did pathlib.Path(os.path.join(...)). The sibling Download.path() still wraps correctly via Artifact.path_after_finished(). Video.path() is the only consumer of absolute_path.

mypy does not catch it: ChannelOwner.__init__ takes a bare initializer: Dict, so absolute_path infers as Any and satisfies any declared return type. mypy playwright reports the same 2 pre-existing errors before and after.

Fail-before, real Chromium recording a real video:

assert isinstance('C:\\...\\page@a808d21e...webm', Path)
E   assert False

After: tests/async/test_video.py 5/5 and tests/sync/test_video.py 6/6 pass. Added the isinstance assertion to both so the contract is guarded; every existing assertion wrapped in str(...), which is why this stayed invisible.

pre-commit run --files on the three files: 15 hooks, 0 errors.

🤖 Generated with Claude Code

Video.path() is annotated -> pathlib.Path and the generated API documents it as such, but it returned Artifact.absolute_path, which is the raw initializer string. The wrap was dropped in 0c70108 when Video moved onto the Artifact abstraction; the sibling Download.path() still wraps via Artifact.path_after_finished().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant